deny UNINSTALL EXTENSION if indexes exist#508
Merged
villagestevers merged 2 commits intoMay 15, 2026
Merged
Conversation
Stub returns false; four gtest cases pin the predicate. Real implementation in the next commit.
Mirrors check_for_columns_of_extension: RESTRICT-only, no MarkForDeletion. Closes the custom_indexes enumeration gap from the villagesql#392 audit.
pyshx
changed the title
villagestevers
approved these changes
May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a RESTRICT-only check in
remove_extension_from_victionarythat blocksUNINSTALL EXTENSIONwhen any rows for that extension remain invillagesql.custom_indexes. Mirrors the existingcheck_for_columns_of_extensionandcheck_for_sp_params_of_extensionpattern — same shape, same RESTRICT semantics (no auto-cascade, noMarkForDeletionof system-table rows at uninstall).The gap surfaced while auditing #392. The functional-index-on-custom-column case (what #392 literally asks about) is already blocked today via the column check; what wasn't enumerated was
custom_indexesitself.A note on reachability: this gap isn't reachable through SQL today —
CREATE INDEX ... USING EXTENDED(...)parses but fails before writing any row (see the--error ER_VILLAGESQL_GENERIC_ERRORinmysql-test/suite/villagesql/index/t/create_index.test). This check exists defensively so it's in place before the CREATE INDEX wiring lands.Related Issue
Part of #392. See the audit comment on the issue for the full per-object analysis and the other follow-ups.
How was this tested?
Four new gtest cases in
unittest/gunit/villagesql/extension_uninstall_checks-t.cc:CustomIndexPreventsExtensionUninstall— the positive caseOtherExtensionsIndexDoesNotPreventUninstall— name discriminatorVersionMismatchDoesNotPreventUninstall— version discriminatorNoIndexesIsAllowed— empty-list happy pathTDD shape: the first commit ships a stub returning
falsesoCustomIndexPreventsExtensionUninstallfails (proves the predicate is wired through). The second commit replaces the stub with the real enumeration loop and adds the call site insql_extension.cc; all four tests then pass.Full village suite green:
A functional mysql-test (
custom_index_prevents_extension_uninstall.test) will be added whenCREATE INDEX ... USING EXTENDEDis end-to-end through VEF. Until then the unit tests pin the predicate — there's aTODO(villagesql-indexing)at the top of the test file noting this.Checklist